CNTRLPLANE-3641: add restructure-commits and rebase GHA workflows - #8770
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
📝 WalkthroughWalkthroughTwo new GitHub Actions workflows are added: Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
bryan-cox
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@bryan-cox: This pull request references CNTRLPLANE-3641 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/rebase.yaml (1)
68-71: ⚖️ Poor tradeoffSupply chain risk: unpinned installer script.
The Claude Code installation fetches and executes a script from
https://claude.ai/install.shwithout checksum verification. If the remote script is compromised, it could execute arbitrary code in the CI environment. This is a common trade-off for convenience, but worth noting given the workflow's elevated permissions.Consider documenting this accepted risk or pinning to a specific Claude Code version if available.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/rebase.yaml around lines 68 - 71, The "Install Claude Code" step downloads and executes an installer script from https://claude.ai/install.sh without verifying its integrity, creating a supply chain risk if the remote source is compromised. Either add checksum verification to the curl command by capturing the script hash and comparing it against a known value before execution, or investigate if Claude Code provides a pinned version installation method and use that instead. If accepting this risk is intentional due to convenience trade-offs, add a comment explaining the accepted risk and why it was chosen despite the security implications.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/rebase.yaml:
- Around line 44-51: The "Select token for PR repo" step has a template
injection vulnerability where steps.pr.outputs.repo is directly interpolated
into the bash script conditional. To fix this, move the GitHub context variable
steps.pr.outputs.repo into an environment variable before the run block, then
reference the environment variable within the bash script instead of using
direct template expansion with double curly braces. This prevents potential
command injection if the repository name contains special characters.
In @.github/workflows/restructure-commits.yaml:
- Around line 44-51: The bash conditional in the "Select token for PR repo" step
contains a template injection risk by directly interpolating
steps.pr.outputs.repo using ${{ }} syntax. Instead, set the repository name as
an environment variable in an env section at the step level, then reference that
variable in the bash conditional using standard shell variable syntax
($variable_name) to safely evaluate the condition.
---
Nitpick comments:
In @.github/workflows/rebase.yaml:
- Around line 68-71: The "Install Claude Code" step downloads and executes an
installer script from https://claude.ai/install.sh without verifying its
integrity, creating a supply chain risk if the remote source is compromised.
Either add checksum verification to the curl command by capturing the script
hash and comparing it against a known value before execution, or investigate if
Claude Code provides a pinned version installation method and use that instead.
If accepting this risk is intentional due to convenience trade-offs, add a
comment explaining the accepted risk and why it was chosen despite the security
implications.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 779621d1-f840-40f8-a9e5-59c5e597de20
📒 Files selected for processing (2)
.github/workflows/rebase.yaml.github/workflows/restructure-commits.yaml
|
/area ai |
openshift-ci
Bot
added
area/ai
Add two new GitHub Actions workflows triggered by PR comments: - /restructure-commits: invokes the repo's restructure-commits command via Claude to reorganize branch commits by component - /rebase: uses Claude to rebase the PR onto latest main, resolve any conflicts, and force push Both follow the same pattern as the address-review-comments workflow with concurrency grouping, fork token handling, and GCP WIF auth. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ff1f5c9 to
c2daac2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/rebase.yaml (1)
70-73: 💤 Low valueSupply chain risk: piping remote script to bash.
The Claude Code installation fetches and executes an external script without integrity verification. If
claude.ai/install.shis compromised or modified, malicious code would run in CI. Consider pinning to a known-good version or checksum.🛡️ Proposed fix with checksum verification
- name: Install Claude Code run: | - curl -fsSL https://claude.ai/install.sh | bash + curl -fsSL https://claude.ai/install.sh -o /tmp/install.sh + # TODO: Add checksum verification when Claude publishes checksums + # echo "EXPECTED_SHA256 /tmp/install.sh" | sha256sum -c - + bash /tmp/install.sh echo "$HOME/.local/bin" >> $GITHUB_PATH🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/rebase.yaml around lines 70 - 73, The Claude Code installation step in the workflow downloads and executes a remote script without verifying its integrity, creating a supply chain security risk. Modify the installation logic to add checksum or signature verification for the downloaded script before execution, or pin the installation to a specific version tag rather than piping an unverified remote script directly to bash.Source: Coding guidelines
.github/workflows/restructure-commits.yaml (1)
95-95: 💤 Low valueConsider reducing
--max-turnslimit.The
--max-turns 200is significantly higher than the rebase workflow's limit of 50. While restructuring commits may require more iterations, 200 turns could lead to unexpectedly long/expensive runs or allow more opportunity for unintended behavior. Consider whether a lower limit (e.g., 75-100) would suffice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/restructure-commits.yaml at line 95, The claude command invocation in the restructure-commits workflow has --max-turns set to 200, which is significantly higher than the 50 turns limit used in the rebase workflow. Reduce the --max-turns parameter from 200 to a more reasonable value like 75 or 100 to prevent unexpectedly long and expensive runs while still allowing sufficient iterations for commit restructuring. This balances the need for multiple turns with the risk of unintended behavior and excessive resource consumption.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/restructure-commits.yaml:
- Around line 82-95: The Claude invocation in the Restructure commits step uses
the -p "/restructure-commits" flag to load a command file from the checked-out
PR branch, which creates a prompt injection vulnerability since a malicious PR
contributor could modify this file. Instead of referencing an external command
file, inline the actual prompt content directly in the workflow file by removing
the -p "/restructure-commits" parameter and providing the prompt text inline
using Claude's input mechanism (such as via echo piping, heredoc, or input
flags). This ensures only the trusted, hardcoded prompt content is executed
regardless of what the PR contains.
---
Nitpick comments:
In @.github/workflows/rebase.yaml:
- Around line 70-73: The Claude Code installation step in the workflow downloads
and executes a remote script without verifying its integrity, creating a supply
chain security risk. Modify the installation logic to add checksum or signature
verification for the downloaded script before execution, or pin the installation
to a specific version tag rather than piping an unverified remote script
directly to bash.
In @.github/workflows/restructure-commits.yaml:
- Line 95: The claude command invocation in the restructure-commits workflow has
--max-turns set to 200, which is significantly higher than the 50 turns limit
used in the rebase workflow. Reduce the --max-turns parameter from 200 to a more
reasonable value like 75 or 100 to prevent unexpectedly long and expensive runs
while still allowing sufficient iterations for commit restructuring. This
balances the need for multiple turns with the risk of unintended behavior and
excessive resource consumption.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b6111972-575b-412f-9b3a-b3875e838d08
📒 Files selected for processing (2)
.github/workflows/rebase.yaml.github/workflows/restructure-commits.yaml
|
/verified later @bryan-cox Unfortunately, we can't test these until they merge 😞 |
openshift-ci-robot
added
verified-later
verified
|
@bryan-cox: This PR has been marked to be verified later by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
|
@bryan-cox: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
What this PR does / why we need it:
Adds two new GitHub Actions workflows that can be triggered by PR comments:
/restructure-commits— invokes the repo's.claude/commands/restructure-commits.mdcommand via Claude to reorganize branch commits into logical, component-based commits matching HyperShift's architecture./rebase— uses Claude to rebase the PR branch onto the latest main, resolve any merge conflicts, and force push.Both workflows follow the same pattern as the existing
address-review-commentsworkflow: concurrency grouping per PR number, member/owner/collaborator authorization, community fork token handling, GCP WIF authentication for Vertex AI, and the same tool dependency setup.Which issue(s) this PR fixes:
Special notes for your reviewer:
Both workflows are modeled directly after
.github/workflows/address-review-comments.yamlwith minimal changes to the trigger command and Claude prompt.Checklist:
Summary by CodeRabbit